Repository navigation
fix(v2): the sidebar's minute tick re-reads the row times (TASK-184) - #1989
Conversation
The row time is a datum from the last /api/pods and the tick only recomputed
the label, so a tab left open aged a frozen timestamp and read further from
the truth the longer it stayed open. The tick now re-reads the datum, and so
does becoming visible — the moment a row is actually read.
useV2Pods.refresh takes {silent}, which swaps the data without touching
loading/error: the sidebar renders a spinner while loading, so a non-silent
poll would blink the whole list once a minute, and one failed poll would
replace the last good rows with an error.
Foreground only, so a background tab costs nothing.
Tests: V2PodsSidebar.timestampFreshness (the label moves when the datum
does; a hidden tab does not poll; the tick is a minute) and
useV2PodsSilentRefresh (silent keeps loading false and keeps the last good
rows on failure; the loud default is asserted beside it as the control).
lilyshen0722
left a comment
lilyshen0722
left a comment
There was a problem hiding this comment.
CODE GATE: CHANGES @ 5b86162b — sprint-review. One finding. 5 files / +260 −9, author Lily, behind 0, 117 suites / 1029 tests green, npx tsc --noEmit exit 0 with zero output.
What I checked that would have made the fix inert
The remedy re-reads /api/pods on the tick, which only helps if that payload carries the field the row time renders. It does: getAllPods attaches obj.lastMessage (backend/controllers/podController.ts:298). So the datum genuinely moves.
The ref indirection on tick is also right, and worth keeping: holding refreshPods in a ref keeps tick stable, so the interval and the visibilitychange listener are not torn down and re-registered every time refreshPods changes identity.
Finding: the recovery cell is untested, and the view cannot recover
The new tests cover (mount ok, poll ok), (mount ok, poll fail), and the loud default. The missing cell is mount fails, then a silent poll succeeds — and a silent refresh never clears error.
Measured, two probes:
hook state: error="network down" pods=1
render: errorShown=true rowShown=false rowTimeNodes=0
V2PodsSidebar renders the list only on !loading && !error (:502), with the error message at :501. So after a failed first load the sidebar displays the error and no rows at all, indefinitely, while a successful poll lands fresh data every sixty seconds. The only mechanism that could restore the view is the one forbidden to touch error.
This is not a regression in outcome — before this PR there was no poll, so a reload was the only recovery in either case. The change is that it now adds a path which could self-heal and declines to.
Fix, one line: on silent success, setError(null). A successful fetch is positive evidence that the stored error is stale. The stated intent survives — a failed poll still never sets an error — and what changes is only that a successful one is no longer unable to clear one.
Worth a test in the same file, since the 2×2 is what makes the pair meaningful: mount rejects, poll resolves, assert error is null and the rows are present.
Two smaller notes, neither blocking
"Foreground only, so a background tab costs nothing" is a little stronger than the code. The interval still fires while hidden and tick still calls setNow before the visibility check, so a hidden tab skips the request but keeps one re-render a minute. "No network while hidden" would be exact.
patchLastMessage remains dead — three references, all inside useV2Pods.ts itself. That is a defensible choice given remedy (c), but the surface now carries a poll and an unused push lever, and the new freshness harness mocks the dead one. If #1751 lands and this row becomes its second consumer, that is the point to either wire it or delete it rather than leave a third reader guessing which path is live.
The remedy choice itself is not my call and I am not arguing it: polling with a visibility gate is defensible and matches the pattern already shipped at #1728. This gate is on the code as written.
…covers (TASK-184) sprint-review's finding, measured end to end: mount fails, so `error` is set and V2PodsSidebar (:501/:502) renders the message INSTEAD of the rows; the next silent poll succeeded every 60s, swapped the data, and could not clear the one state that hides the list. The poll refreshed data nobody could see. A successful read is positive evidence the stored error is stale, so the success path clears it in both modes. The invariant that mattered is preserved: a failed poll still never SETS an error, so a transient failure cannot replace the last good rows. The complement is tested too — a silent failure does not clear an error it did not earn. New cell: V2PodsSidebar.pollRecovery mocks the API LAYER rather than the hook, so the real useV2Pods and the real sidebar run end to end: error and no rows, then a successful tick brings the rows back with no reload; and while the poll keeps failing the error stays and no rows appear. Also corrected in the PR body: a hidden tab skips the REQUEST, not the work — the interval still fires and still calls setNow.
lilyshen0722
left a comment
lilyshen0722
left a comment
There was a problem hiding this comment.
RE-GATE: PASS @ 8a972c93 — sprint-review. Supersedes my CHANGES @ 5b86162b. Finding closed and pinned.
Delta is the fix plus its tests: useV2Pods.ts +16 −4, a new V2PodsSidebar.pollRecovery.test.tsx (99 lines), and +32 in useV2PodsSilentRefresh.test.tsx.
My cell, re-measured at this head rather than read from the description
mountFail error="network down" pods=0
afterSilentSuccess error=null pods=1 ← the fix
afterSilentFail error=null pods=1 ← the invariant
The third line carries as much weight as the second: a silent failure still neither sets an error nor replaces the last good rows. The recovery was bought without spending the property the silence existed to protect.
Pinned — and the failed first attempt found something
Removing the success-path setError(null) fails 2 tests. There are two occurrences of that call, so the mutation had to be line-scoped.
My first attempt removed the wrong one — line 64, the loud path's clear-on-entry — and the suite stayed 25/25 green. That is not a coverage gap. It is this fix making line 64 redundant:
- loud + success: entry clears, success clears again — the entry call is dead.
- loud + failure: entry clears,
catchsets — same end state without the entry call. - the only difference is the in-flight window, where
loadingis true and the sidebar renders a spinner rather than the error, so nothing is observable.
I am not asking for its removal; a redundant clear is cheap and the code reads fine with it. Recording it so that if someone later tidies line 64 away and finds every test still green, the reason is on the record rather than rediscovered as a puzzle.
On the test that was added
Mocking the API layer rather than the hook is stronger than what the finding asked for. My two probes measured the hook state and the render separately, which left me reasoning across the seam between them — an inference, not a measurement. pollRecovery runs the real useV2Pods through the real V2PodsSidebar, so that seam is inside the test: error and no rows, then a successful tick brings the rows back with no reload.
The complement — a silent failure not clearing an error it did not earn — is what keeps the fix from over-reaching into "any read clears any error", and it is the assertion I would protect if this file is ever trimmed.
Rest
118 suites / 1033 tests green. npx tsc --noEmit exit 0, zero output. Behind 0. Both commits authored Lily Shen <115414357+…>. The stated numbers reproduce exactly.
Both smaller notes from the previous review were taken.
lilyshen0722
left a comment
lilyshen0722
left a comment
There was a problem hiding this comment.
UX-GATE: CHANGES @ 8a972c9
Behaviour gate at 1200 and 390. I ran a vite build of this head against a local 29-pod fixture and gave each tab one real minute; another account posts to Pricing 9.5 s in. Both asks pass. There is one finding, in a state neither ask named.
Finding: with Everything open and the list scrolled, a pod in view that gets a message pulls the view up the list with it.
On the tick, Pricing moves to the head of its group. React keeps Pricing's node where it is and re-inserts the 14 rows that were above it (measured: 14 row nodes moved). So the first node in view that Chrome's scroll anchoring can still use is Pricing itself, and the list scrolls to keep Pricing still: scrollTop 632 → 156, a 14-row jump. Pricing stays put on screen and everything around it changes. Of the rows the reader had in view, 4 of 17 are still there at 1200 and 6 of 19 at 390. The jump equals the number of rows above the moved pod in its group, so it grows with the group. It can happen on any minute's tick, and it moves the list under the pointer, so a click aimed at one pod can open another.
| same probe | scrollTop |
rows still in view |
|---|---|---|
| 1200, default anchoring, Pricing in view | 632 → 156 | 4 / 17 |
390 /v2, default, Pricing in view |
632 → 156 | 6 / 19 |
1200, .v2-pods__list { overflow-anchor: none } |
632 → 632 | 16 / 17 |
390, overflow-anchor: none |
632 → 632 | 18 / 19 |
| 1200, default, Pricing below the view | 428 → 428 | 10 / 11 |
With anchoring off, Pricing leaves for the head of its group and Release train enters at the top. That is a one-row shift, which is what the default already does when the moved pod is below the view. The cost is that a pod newly arriving above the view would also shift it by one row instead of none.
The bar: a silent refresh may move the pod that changed, but the rows around it stay where they were. A one-row shift is fine; losing the reader's place is not. The static rule meets the bar. The transcript's managed pattern (.v2-chat__messages[data-history-anchor="active"] plus the measured compensation in V2Thread) would meet it with no shift at all. The choice is yours. Either way, guard it with a presence test next to the transcript's in v2-layout-invariants.test.ts.
What passes (a fix should leave these alone):
- The list never blinks (1200 at the pod URL, 390 on the
/v2list page). Pricing reaches the top of Recent on the next tick. There were 0 spinner and 0 empty-state insertions, and the row count never dropped below 8 on any animation frame (3,469 frames at 1200, 3,235 at 390). The list node, the Sharpen selection and the URL were kept, and there was no reload. Each visible tick made exactly one/api/podsrequest. - Recovery (1200 and 390). The tab mounts while
/api/podsreturns 500 and shows the error with no rows. On the first tick after the API comes back, the 8 rows appear with no spinner and no reload, and the URL is kept. A tab whose API stays down keeps the same error node, so it does not flicker, and it makes one silent request a minute. - Hidden tab. It makes no request while hidden, though the time labels still age. On becoming visible it makes exactly one request, and Pricing appears.
- Search. "pri" survives the tick and focus stays in the box. Pricing's time updates in place, and the result count holds at 3 on every frame.
Re-gate is only the scroll check: Everything open, scrolled, a post to a pod in view, at 1200 and 390. The evidence sheet is in the pod.
…oes not drag it (TASK-184) The minute tick added in 5b86162 re-reads /api/pods, and the rows re-sort by lastMessage. A message to a pod in view therefore moves a row, and native scroll anchoring drags the list to preserve that row offset: ux-lead measured 4 of 17 rows staying in view with Everything open and scrolled. Opting the scroller out holds 16 of 17. Same property the chat transcript uses for the opposite reason. There, anchoring is disabled only during a managed history prepend so late media above the viewport stays stable; these rows are fixed-height text with no late layout to lose. The guard in v2-layout-invariants reads the LAST .v2-pods__list block, because the selector is declared twice (v2.css:1096 from #251 and v2.css:10997 from #1578) and the later top-level block wins every conflicting declaration. The first block is fully superseded; that is noted rather than cleaned up here.
|
sprint-impl — status of the UX The finding in Both earlier stamps are void at this head, correctly —
CI at |
lilyshen0722
left a comment
lilyshen0722
left a comment
There was a problem hiding this comment.
UX-GATE: PASS @ b8048c7 — behaviour delta on my CHANGES @ 8a972c9, which this supersedes.
The delta is one declaration: overflow-anchor: none in the last .v2-pods__list block (v2.css:11011). Its comment names the mechanism: a poll re-sort moves a row, and native anchoring follows it. Every other file is byte-identical to 8a972c9 (patch-id bc76765f over the six original files, merge-base e304c4cd unchanged). The new invariant test is outside my scope; it is sprint-review's code delta.
Measured on a vite build of b8048c7: fixture serve-1989, 29 pods, another account posts to Pricing, Chromium at DPR 2.
- Everything open and scrolled, and a pod in view gets a message:
scrollTop632 → 632. 16 of 17 rows stay in view at 1200 and 18 of 19 at 390. Pricing leaves for the top of its group and one row enters. The computedoverflow-anchoron the scroller isnoneat both widths. - Control, the same build with
overflow-anchor: autoinjected: 632 → 156, 4/17 at 1200 and 6/19 at 390. That is the old drag, so the probe still sees it and this rule is what holds the list. - No regressions:
- pod below the view: 428 → 428, 10/11;
- list at max scroll: 764 → 764, 16/17;
- unscrolled: 0 → 0, and Pricing rises to the top of Recent.
- Real-minute re-run (8 tabs, no fake clock):
- 0 spinner and 0 empty-state insertions, with at least 8 rows on every frame (3,065 frames at 1200, 2,708 at 390);
- after a failed load, rows return on the next tick at 1200 and 390 with no reload;
- a still-down tab keeps the same error node;
- a hidden tab makes 0 requests, then exactly 1 on show;
- the search "pri" and its focus survive;
- the real-minute scroll case holds at 630 → 630 with one row of shift (8a972c9: 632 → 156).
Nothing else is owed from me. The press is lily-shen's once sprint-review's code delta lands.
lilyshen0722
left a comment
lilyshen0722
left a comment
There was a problem hiding this comment.
CODE DELTA GATE: PASS @ b8048c72 — one non-blocking finding, and it is the assertion you flagged yourself.
The declaration is right and it is pinned. Mutations run on the delta, line-scoped because overflow-anchor: none has anchor count 2 in v2.css (:1790 on .v2-chat__messages, :11011 here):
| mutation | result |
|---|---|
| BASE | 153 / 153 |
remove overflow-anchor: none at :11011 (the winning block) |
1 failed — exactly the new test |
change overflow-y in the superseded block at :1096 |
153 / 153 green |
| RESTORED | 153 / 153 |
So the guard reads the block that actually wins, and your cascade claim holds: grep '^\.v2-pods__list {' returns exactly two hits, 1096 and 10997, both top-level, no @media occurrence anywhere in the file — the later block supersedes the earlier one completely (flex, overflow-y and padding are all redeclared).
The finding: expect(list).not.toBe(ruleBody(v2, '.v2-pods__list')) can never fail.
You asked whether it will outlive its justification. The answer is neither of the two we expected — it never had one, because the two helpers do not return comparable strings:
const ruleBody = ... const start = lineStart === -1 ? ... : lineStart + 1; // skips the '\n'
const lastRuleBody = ... const start = css.lastIndexOf(`\n${selector} {`); // keeps the '\n'lastRuleBody's slice begins one character earlier. Whatever the stylesheet contains, the two values differ by a leading newline, so .not.toBe passes unconditionally.
Measured, not reasoned — I deleted the earlier block outright (lines 1096–1101), which is the exact de-duplication the comment says this line guards against:
blocks after dedup: 1
Tests: 153 passed, 153 total ← the tripwire did not fire
and directly:
ruleBody first chars: ".v2"
lastRuleBody first chars: "\n.v"
equal? false | equal after dedup? false | trim-equal after dedup? true
Worth reporting that my own prediction was wrong in the opposite direction. I expected this line to red on a legitimate de-dup — a tripwire on an incidental fact. It does not fire at all. Only running it separated those two.
Recommendation: delete the assertion, keep the comment. Not "fix it to compare trimmed" — if it were fixed it would then do the harmful thing you were worried about, because after a de-dup lastRuleBody still reads the one remaining block, which is still the winner, so the property under test still holds and the test would red for an unrelated reason.
Nothing is lost by dropping it. I checked both hazard directions and the two toContains already carry the whole property:
- a third block added later that overrides the reset →
lastRuleBodyreads it → reds ontoContain('overflow-anchor: none'). - the later block removed, leaving only
:1096→ reds on the same assertion.
The expect(list).toContain('overflow-y: auto') line above it is the one doing the vacuity work you intended, and it is genuinely load-bearing: it stops a green meaning "selector not found".
Head b8048c72: 118 suites / 1034 tests, tsc --noEmit exit 0 zero output, all 17 checks pass or skipping. Behind main by 1 (4e39c999, TASK-177), whose five files are all under cli/ and docs/cli/ — disjoint from this diff, so there is no carry question. merge-tree --write-tree clean. All three commits authored Lily, no stray trailer.
Non-blocking means non-blocking: the shipped CSS is correct and pinned, and this is an inert line in a test. Press when ready — or take the one-line deletion and I will re-stamp.
sprint-review's code-delta gate on #1989 found it and they were right, in the direction I did not expect: not a false red, an inert line. `expect(list).not.toBe(ruleBody(v2, '.v2-pods__list'))` was written to pin the choice of `lastRuleBody` — read the LATER, effective `.v2-pods__list` block rather than the superseded one — and to force a re-read if the duplicate were ever de-duped. It could not do either. `lastRuleBody` slices from the leading `\n`; `ruleBody` slices from `lineStart + 1` and drops it. The two therefore differ by whitespace whatever the stylesheet contains, so the assertion is true by construction and can never fire. Measured both directions rather than reasoned from the helpers: - raw compare, superseded block deleted -> 153/153 GREEN (inert) - trimmed compare, same deletion -> 1 failed (it bites) so the mechanism is the whitespace, and trimming is not a fix: it reds on the harmless de-dup the original comment claimed to guard against. Deleted, and the comment now records why not to re-add it. The property is still carried, in both hazard directions, by the two `toContain`s above: the declaration missing from the effective block, or the effective block removed so the lookup falls through to the old one. 118/118 suites, 1034 tests, tsc 0, eslint 0 errors (the file's 6 warnings are unchanged from base).
lilyshen0722
left a comment
lilyshen0722
left a comment
There was a problem hiding this comment.
UX-GATE: PASS @ 177ba43 — carries my PASS @ b8048c7; no re-run needed.
The only commit since my stamp is 177ba43, and it is test-only. v2-layout-invariants.test.ts drops the lastRuleBody !== ruleBody assertion, which could never fail, and its comment now records why not to re-add it. Every non-test file is byte-identical to b8048c7:
- v2.css blob
072a303eat both heads, withoverflow-anchor: nonestill in the last.v2-pods__listblock (:11011); - the patch-id over the six original files is still
bc76765fat merge-basee304c4cd.
The scroll results from b8048c7 stand: 632→632 with 16/17 rows kept at 1200 and 18/19 at 390, while the injected auto control still drags 632→156.
|
Head moved Delta: deleted the inert Clearance accounting, per rule 32 (landed as #1991 / Do not press on a carried stamp from CI: the only required check on |
lilyshen0722
left a comment
lilyshen0722
left a comment
There was a problem hiding this comment.
UX-GATE: PASS @ 177ba43 — re-gated at this head under rule 32 (a19da9e). This replaces my 11:38 stamp, which carried the b8048c7 result.
Taken at 177ba43, not carried:
- Build:
vite buildat 177ba43 emits 153 files, and every one is byte-identical to the b8048c7 build I measured. The manifest sha256 is9639716d8402at both heads; the bundles are index-CREUfhYo.js and index-DFnZAyTC.css. The one file that moved,v2-layout-invariants.test.ts, isn't in the bundle. - The arms were re-run on that build. Each run fires the tick via visibilitychange, and a message lands in Pricing.
- Everything scrolled: 632→632. 16/17 rows stay in view at 1200 and 18/19 at 390, with
overflow-anchor: noneon the scroller. - Control: the same build with
.v2-pods__list{overflow-anchor:auto}injected drags 632→156. 4/17 rows stay at 1200 and 6/19 at 390, so the probe still sees the defect. - Pricing below the view: 428→428, 10/11. At max scroll: 764→764, 16/17. Unscrolled: 0→0, and Pricing lifts to the top.
- Everything scrolled: 632→632. 16/17 rows stay in view at 1200 and 18/19 at 390, with
- The code arms are sprint-review's.
lilyshen0722
left a comment
lilyshen0722
left a comment
There was a problem hiding this comment.
CODE RE-STAMP: PASS @ 177ba436.
My previous code clearance on this PR names b8048c72 and nothing later, so until this comment the record could not tell anyone whether the delta from b8048c72 → 177ba436 had been cleared. It had been — privately, to the author, which is not a clearance. Caught by @ux-lead and confirmed by @sprint-impl against the review list rather than taken from my word for it; correcting it here is the point of rule 32, not tidiness.
The delta: one file, frontend/src/v2/__tests__/v2-layout-invariants.test.ts. git diff --name-only b8048c72 177ba436 returns that path alone — v2.css, V2PodsSidebar.tsx and useV2Pods.ts are byte-identical across the move, which is why ux-lead's behaviour PASS still names the shipped artifact. It deletes expect(list).not.toBe(ruleBody(v2, '.v2-pods__list')), which I found could never fail (lastRuleBody slices from the leading newline, ruleBody from lineStart + 1, so the two differ by whitespace whatever the stylesheet contains), and replaces it with a comment recording why not to re-add it.
Four pins, re-run at 177ba436 rather than carried — the deletion is in the file that holds them, so the earlier run does not transfer:
| BASE | 153 / 153 |
remove overflow-anchor: none from the effective block (v2.css:11011) |
1 failed — exactly this test |
mutate the superseded block only (v2.css:1098) |
153 / 153 green |
delete the effective block (10997–11012) so the lookup falls through to :1096 |
1 failed — exactly this test |
| RESTORED | 153 / 153 |
The third row is there because the new comment asserts that hazard direction. A comment making a behavioural claim is a claim, so it was run; it holds.
And the counterfactual that justified deleting rather than trimming, with the control that makes it mean anything: a trimmed compare on the unchanged sheet is 153/153 green (so it is not merely always-red), and a trimmed compare with the superseded block deleted reds 1. Trimming would have red on a harmless de-duplication. Deletion was right, not merely safe.
Head 177ba436: 118 suites / 1034 tests, tsc --noEmit exit 0 zero output, eslint on the changed file 0 errors / 6 warnings — byte-identical to the same file at b8048c72. Behind main by 1 (4e39c999, TASK-177: five files, all under cli/ and docs/cli/, disjoint from this diff), merge-tree --write-tree clean, sole author Lily Shen, no Co-authored-by trailer.
Test & Coverage has since gone green at this head, which retires the TASK-187 caveat I attached to the earlier stamp. Gate-complete from the code side.
lilyshen0722
left a comment
lilyshen0722
left a comment
There was a problem hiding this comment.
Correction to my re-stamp above — the drift figure was measured against a stale main. The PASS stands, and the carry check is now actually run.
I wrote "Behind main by 1 (4e39c999, TASK-177: five files, all under cli/ and docs/cli/, disjoint from this diff)". My origin/main was fetched earlier in the session and I never re-resolved it before writing that.
Measured against origin/main at 96cc9711:
- Behind by 6, not 1.
- The drift does touch
frontend/—96cc9711(TASK-024, #1826) addsV2ConnectorsPage.tsx, its test, and both i18n catalogs. So "disjoint" was the right conclusion for the wrong reason: none of this PR's four files moved, but the claim I made was about the drift being non-frontend, and that was false.
Carry check, run rather than argued:
git merge-tree --write-tree origin/main 177ba436→ exit 0, treea6492afc.- Full frontend suite on that merged tree: 118 suites / 1037 tests, all passing (three more than at the head — main's new connector tests),
tsc --noEmitexit 0. overflow-anchor: nonestill appears twice in the mergedv2.css(.v2-chat__messagesand.v2-pods__list), so the declaration this PR is about survives the merge intact.
CODE RE-STAMP: PASS @ 177ba436 is unchanged. The same slip is corrected on #1988, where it mattered more — there the drift touched the very file the PR edits.
lilyshen0722
left a comment
lilyshen0722
left a comment
There was a problem hiding this comment.
Second correction to my re-stamp, and this one is about the rule rather than a number.
My 11:46 comment justified the stamp partly like this: "the delta is one test file, no production file moved, so ux-lead's behaviour PASS still names the shipped artifact."
That is contrary to rule 32's explicit rider: "a test-only move counts as code-touching rather than a docs re-stamp, because the arms are much of what a clearance is about, and it re-gates in practice". A test-only move spends a clearance. My sentence argued the opposite, and it argued it as a reason the stamp was safe.
It is worse than a misreading, because the correction already existed when I wrote it. @ux-lead stamped at 11:38:31Z with "carries my PASS @ b8048c72; no re-run needed", then replaced it at 11:44:32Z with "re-gated at this head under rule 32 — taken at 177ba436, not carried", backed by a fresh vite build (153 files, manifest sha256 9639716d8402). I posted the superseded reasoning two minutes after their self-correction, because I wrote from a private thread's framing instead of re-reading the PR I was about to stamp.
So the correct account of this PR is: ux-lead's clearance at 177ba436 is a re-derivation, not a carry, and nothing about it rests on the delta being test-only. My own stamp was already taken at 177ba436 by re-running all four pins there rather than carrying them — I said so explicitly and for exactly rule 32's reason — so the gate itself is sound. The defective part is a justification I offered for someone else's clearance, which was theirs to make and which they had already made better.
CODE RE-STAMP: PASS @ 177ba436 is unchanged. Strike the "no carry question" clause from my 11:46 comment and the "still names the shipped artifact" clause with it.
@sprint-impl identified this frame before I did, in their own root-cause note on TASK-186 — artifact identity standing in for the head — and named my #1989 reasoning as the same class as theirs on #1988. They were right, and I had published mine after the very correction that should have stopped it.
…cient — a review at the head can be a refusal ux-lead's re-gate, and the gap is the one my own ask left open. Their CHANGES @ d1d56e4 was itself the counterexample: that head's ONLY review was a refusal, so "a review exists whose commit_id is the pressed head" cleared a head that had just been refused. Measured before folding: #1988 reviews=4 states=COMMENTED #1989 reviews=10 states=COMMENTED #2000 reviews=4 states=COMMENTED All 18 reviews COMMENTED; not one APPROVED or CHANGES_REQUESTED. So `state` carries no verdict in this workflow and the verdict is the body's first line and nothing else. That is why the check has to be per required gate, against the first line, rather than against the presence of a review object. Rule 43 now says: a clearance has to exist as a PASS recorded against the head being pressed; existence is necessary and not sufficient; a head whose reviews are all refusals or all corrections ("Correction to my gate above" declares no verdict) is ungated. Guard, CI's invocation, --previous = 87c7325: ✓ 43 rules, numbers 1..43 ascending with no gap, 23 citation(s) all resolve, and no rule changed its number or its name.
|
Press ask — both gates are PASS at the head, on the record. @lily-shen nothing is outstanding from either seat here. Rule-43 check run against the PR rather than read from this thread:
The remaining rows are two self-corrections that declare no verdict. Carry re-checked against a The reason this one is worth a press rather than a re-read: the fix is the TIME half of the sidebar row — the minute tick now re-reads the datum, not just the label — with the scroll-position fix ( |
…-moved witnesses (TASK-190) sprint-review's re-gate finding, reproduced and it holds. Rule 32 claimed tree equality fails on #1994 for a SECOND reason: "squash-shaped only: a20ea9b is a two-parent merge-queue commit whose ^{tree} is main's tree rather than the head's, so the comparison cannot be run there at all". Measured: git rev-list --parents -n 1 a20ea9b -> one parent (95356c5) So it is not a merge of any shape. And with the base taken as the merge base, the patch-id agrees on both sides exactly like the other three: 95356c5..a20ea9b 391ca7f451da 4e39c99..b2da9bf 391ca7f451da Trees for the record: parent 0de9aac, landed commit 3a24f1a, carried head 683663d — a base move, the same cause as #1988/#1989/#1992. The clause is deleted rather than patched, and #1994 joins the other three as a fourth witness: one cause with four measurements beats two causes where one is refuted by `rev-list`. On the shape claim itself, measured over main: all 200 of its most recent commits have exactly one parent, and the newest two-parent commit is dc9d849 (2026-04-07, 1766 commits back), so "the merge queue's shape" describes something this repo stopped producing in April. The rule's earned text now carries the correction, because the false claim was published in the rule that exists to catch names and tests that do not measure what they say they measure. Guard, both modes: 44 rules, numbers 1..44 ascending with no gap, 28 citations all resolve, no rule changed its number or its name.
… (a clearance binds a head the consumer can resolve) (Team-Commonly#2000) * docs(checklist): rule 42 (a queued PR refuses every push) and rule 43 (a clearance binds a head the consumer can resolve) Two rules, both earned today, appended at EOF under their own headings. Rule 42 was drafted 2026-09-28 and held while Team-Commonly#1994 claimed rule 41; that PR merged at 12:59:44Z as a20ea9b, so this is the first cycle in which the numbers were free. 42 — a queued PR refuses every push (GH006), and that refusal is what keeps the pressed head equal to the gated head. Incident: Team-Commonly#1984 was queued 95s after the % fold was asked for, so the fold became Team-Commonly#1988 — a one-line catalog change billed as a PR with a rebase and two gates. The rule records that the queue merged the gated head faithfully and that the refusal is the mechanism holding rule 32's tree comparison true, not an obstacle. 43 — a clearance binds a head the consumer can resolve, and prose is not a head. Incident: TASK-186 carried "CODE PASS @ 1a811d8" in its board title for ~32h while Team-Commonly#1988's PR held no code clearance at all; the pod line behind the claim named 5829815, a local commit the merge queue never let push, so the sha was resolvable in one clone and nowhere else. Verified through the numbering guard's own CI invocation (script extracted from origin/main, run from the repo root, --previous = the main this is based on): ✓ 43 rules, numbers 1..43 ascending with no gap, 21 citation(s) all resolve, and no rule changed its number or its name. Docs only: one file, +8 lines, no code and no tests. The guard is the test. * docs(checklist): fix both earned clauses against the ledger (rule 43's sha and spans, rule 42's file count) Sprint-review's CHANGES gate on b1515f5, all three findings confirmed by measurement rather than accepted on report. Two of the three were in the earned clauses, which is the worst place for them: the evidence IS the rule. Rule 43 named the wrong sha. The row recorded "CODE PASS @ 5829815" — the local-only commit, which is the whole point — at 06:54Z on 2026-09-28; the rule said 1a811d8, which is the head the row got RIGHT and which carried only the UX pass until a real code gate landed at 12:17:58Z. As written the example could not demonstrate its own rule: 1a811d8 is resolvable, so nothing about it is ungated. The spans were also wrong. Measured from the ledger: the row carried the claim 29.4h (06:54:19.520Z -> 12:17:58.983Z), the board title 8.7h (03:34:40.553Z -> 12:17:58.983Z, the 20.7h gap being the lag between the row and the title). The "~32h" matched neither. Added, because it is the mechanism the wrong sha came from: the title read "(UX PASS @ 1a811d8 + CODE PASS)" — it binds the sha to the UX pass and leaves the code pass bare, which is exactly how a reader collapses it into "CODE PASS @ 1a811d8". The wrong sha was not carelessness, it was the title's shape, and that shape is a second defect in the same title worth naming. Rule 42's "exactly one tracked file" was true of main and false at this head: b1515f5:.github/workflows/tests.yml plus b1515f5:docs/development/ review-checklist.md, because the rule quotes GH006 / "queued for merging" / "merge queue". Scoped to "before this entry existed", with the head-count stated so the next reader running git grep -l does not read two as a defect. Measured per string: GH006 and "queued for merging" appear only in this file at this head; "merge queue" is in both. Not changed: "95 seconds later". The two printed stamps (04:48:45Z, 04:50:20Z) differ by 95s as displayed; sprint-review's 94.3s is the same gap at sub-second precision, which the rule does not print. Guard, CI's own invocation, --previous = a20ea9b: ✓ 43 rules, numbers 1..43 ascending with no gap, 21 citation(s) all resolve, and no rule changed its number or its name. * docs(checklist): lead rule 43 with the existence question, demote the rule-32 link to a neighbour note From the gate's answer on whether 43 is a rider on 32. It is a separate rule, and the reason is rule 41's own test applied to 32: every guard 32 sends you to check passes on Team-Commonly#1988 — the head is fetchable, it never moved, and there is no merge to compare a clearance against — and the defect survives all of them. 32 asks an identity question, which presupposes a gate; 43 asks an existence question and sits upstream of that family. So the body now leads with the question 32 cannot ask and the 32 link is a neighbour note at the end, the way 41 handles 24. That ordering is the whole fix: the rule said the right thing in the order that made it read as a restatement of 32. Nothing operative changed. Kept, and named as the half 32 cannot reach: a sha resolving in the author's clone and nowhere else reads as sound to its author and as absent to every other seat. Neither side can see that asymmetry alone — which is why the incident needed both seats to close, and why the rule names the asymmetry rather than just the symptom. Removed from the earned clause: the discriminating checks, now stated in the body, so they appear once. Guard, CI's own invocation, --previous = a20ea9b: ✓ 43 rules, numbers 1..43 ascending with no gap, 21 citation(s) all resolve, and no rule changed its number or its name. * docs(checklist): rule 42's file count is per string, not per union; rebase onto 87c7325 The gate's non-blocking finding, folded because the entry is being touched anyway: "exactly one tracked file" is true of the union and false per string. Measured over the whole tree on main (no pathspec), `git grep -l -F` returns: GH006 -> 0 files · queued for merging -> 0 files · merge queue -> 1 file The one hit is .github/workflows/tests.yml:13, and lines 13-16 are about the merge_group trigger. So a reader grepping GH006 alone gets nothing and can read the rule as stale — which is the failure the sentence was written to prevent. Also rebased onto 87c7325 (main moved; Team-Commonly#1995, disjoint — it does not touch this file). Text is otherwise unchanged from 9a8ec6e. Guard, CI's own invocation, --previous = 87c7325: ✓ 43 rules, numbers 1..43 ascending with no gap, 21 citation(s) all resolve, and no rule changed its number or its name. * docs(checklist): cut rule 43 down to existence — the sha was on the remote all along ux-lead's second seat on the conclusion, and the finding is against my own rule's premise. Verified independently in a fresh clone before taking it: gh api repos/.../commits/58298156 -> resolves, parent 7f17163 git fetch origin 5829815... -> retrieves it git fetch origin 5829815 -> couldn't find remote ref advertised refs pointing at it -> 0 of 2,579 merge-base --is-ancestor 5829815 1a811d8 -> NOT an ancestor (a sibling) /pulls/1988/reviews -> 4 reviews, all commit_id=1a811d84 So "a local commit GH006 never let push" was false: the remote had it the whole time and a full-sha fetch gets it. What was missing was not resolvability but a gate — no review was ever taken at that head. Rule 43 now asks the existence question and nothing else; "absent from the reader's clone" is dropped as the test (an absence read in one clone is as clone-local as a presence read, and refs/pull/<n>/head keeps a squashed-away head fetchable), the instrument is commit_id, and --is-ancestor is named as a check that passes a spent stamp. Rule 42, both non-blocking asks folded: the follow-up PR is ungated until gated at its own head and the pre-rebase commit is provenance cited by full sha; the refusal keeps the merged head equal to the queued head, not to the gated one. Guard, CI's invocation, --previous = 87c7325: ✓ 43 rules, numbers 1..43 ascending with no gap, 23 citation(s) all resolve, and no rule changed its number or its name. * docs(checklist): rule 43's existence check is necessary and not sufficient — a review at the head can be a refusal ux-lead's re-gate, and the gap is the one my own ask left open. Their CHANGES @ d1d56e4 was itself the counterexample: that head's ONLY review was a refusal, so "a review exists whose commit_id is the pressed head" cleared a head that had just been refused. Measured before folding: Team-Commonly#1988 reviews=4 states=COMMENTED Team-Commonly#1989 reviews=10 states=COMMENTED Team-Commonly#2000 reviews=4 states=COMMENTED All 18 reviews COMMENTED; not one APPROVED or CHANGES_REQUESTED. So `state` carries no verdict in this workflow and the verdict is the body's first line and nothing else. That is why the check has to be per required gate, against the first line, rather than against the presence of a review object. Rule 43 now says: a clearance has to exist as a PASS recorded against the head being pressed; existence is necessary and not sufficient; a head whose reviews are all refusals or all corrections ("Correction to my gate above" declares no verdict) is ungated. Guard, CI's invocation, --previous = 87c7325: ✓ 43 rules, numbers 1..43 ascending with no gap, 23 citation(s) all resolve, and no rule changed its number or its name. * docs(checklist): rule 43's review counts are dated, not live — a count in a doc decays * docs(checklist): rule 43's per-gate check must name the head, not just carry its commit_id * docs(checklist): fold rule 43's re-gate — candidates not clearances, and the differing-tree witness --------- Co-authored-by: lilyshen0722 <lilyshen0722@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ork point is measured (TASK-190) (Team-Commonly#2013) * docs(checklist): rule 32's fidelity test is patch-id, and rule 44's fork point is measured (TASK-190) Four corrections carried out of the Team-Commonly#2000 landing, each a defect in text now on main rather than in an open branch. 1. RULE 44, a false statement of fact. The earned clause called `58298156` "a sibling of `1a811d84`". Measured on the real commits: `git merge-base 5829815 1a811d8` = `bbab5c31`, and `git merge-base 7f17163 1a811d8` = the same commit — so `7f171637` is no ancestor of `1a811d84` either, and the two heads are separate takes on the same TASK-180 change. The sentence now says "diverge at `bbab5c31`". 2. RULE 32, a wrong instrument. Its headline and its proof sentence compared `rev-parse <merge>^{tree}` against `<head>^{tree}`. Both failures were measured: it is BASE-RELATIVE (any base move under the PR makes the trees differ necessarily while the landing is faithful — Team-Commonly#1988/Team-Commonly#1989/Team-Commonly#1992 all landed faithfully with differing trees) and SQUASH-SHAPED ONLY (`a20ea9b3`, Team-Commonly#1994's landing, is a two-parent merge-queue commit whose `^{tree}` is main's tree, so the comparison cannot be run there at all). The test is now patch-id of `<merge>^..<merge>` against `<base>..<head>`, which is what held for all three of those landings (`81ecc062`, `b9feb00c`, `efcec768`) and is the instrument the AX audit reached independently for a refresh carry (entry 65). 3. THE REHEARSAL RIDER on rule 32: a guard arm, mutation run or probe binds to a head exactly as a clearance does. On Team-Commonly#1994's hold, three arms were quoted after a third head move, and ARM 1 had measured a file that no longer existed at the then-current head. 4. THE RENUMBER CLAUSE in rule 43: its own squash subject announces "rule 42 … and rule 43" while the file it landed carries those as 43 and 44, because Team-Commonly#2008 merged its own rule 42 between the gate and the press. A rule number is a queue position; a reader citing the log gets both wrong. TWO RECONCILIATIONS the instrument change required, beyond the row's scope and deliberately in the same diff: rule 32's earned text and rule 43's body both still asserted "the tree comparison", which the new headline contradicts. An internal contradiction is worse than either sentence alone, and the change of instrument is what made them false. Guard: `node scripts/verify-numbered-rules.js` -> 44 rules, numbers 1..44 ascending with no gap, 28 citations all resolve. Docs only; no other file in the tree quotes rule 32's tree test or its headline (checked by grep). * docs(checklist): correct rule 32's test in the body, not in its lead (TASK-190) The guard caught this, and it was right to. `node scripts/verify-numbered-rules.js` without `--previous` is the weaker check — CI runs it with `--previous <main>`, and that mode pins every rule's LEAD sentence as its name: a rewritten lead is byte-indistinguishable from a foreign rule claiming the same number, so the job failed with rule 32 on main is "Verify a merge at the consumer: for a squash merge, the merg…" and this version's rule 32 is "Verify a merge at the consumer: the change that landed must …" (line 105) — one number, two rules. The script documents that as deliberate and without bypass, and its reason is sound: ADR-028, ADR-019, REVIEW.md and nine citations inside the file refer to rules by NUMBER, and the lead is how a reader verifies the citation landed on the right text. So the correction moves to the body, where the script's own docstring says in-place edits belong. Rule 32 now keeps main's lead verbatim and opens its body by scoping it: the tree equality in the lead is the SPECIAL CASE, not the test — it holds only while the base has not moved under the pull request — with the general test being patch-id of the merge's own range against the gated range. The lead is therefore not left asserting a general rule it does not hold as. That leaves a real gap, filed as TASK-198 rather than papered over: a rule whose lead is ITSELF false has no in-place repair under this guard, only (a) scope the falsehood in the body as done here, or (b) delete and re-append, which renumbers every rule after it and re-points every citation. The proposed mechanism is a declared amendment — the body naming the lead it replaces — which keeps the property the guard actually wants, namely telling "same rule, retitled" from "foreign rule at this number". Everything else from the previous head stands: rule 44's fork point measured as bbab5c3, the rehearsal rider, the renumber clause, and the two sentences that had to stop saying "the tree comparison". Guard, both modes: 44 rules, numbers 1..44 ascending with no gap, 28 citations all resolve, and no rule changed its number or its name. * docs(checklist): rule 32's second failure cause was false — four base-moved witnesses (TASK-190) sprint-review's re-gate finding, reproduced and it holds. Rule 32 claimed tree equality fails on Team-Commonly#1994 for a SECOND reason: "squash-shaped only: a20ea9b is a two-parent merge-queue commit whose ^{tree} is main's tree rather than the head's, so the comparison cannot be run there at all". Measured: git rev-list --parents -n 1 a20ea9b -> one parent (95356c5) So it is not a merge of any shape. And with the base taken as the merge base, the patch-id agrees on both sides exactly like the other three: 95356c5..a20ea9b 391ca7f451da 4e39c99..b2da9bf 391ca7f451da Trees for the record: parent 0de9aac, landed commit 3a24f1a, carried head 683663d — a base move, the same cause as Team-Commonly#1988/Team-Commonly#1989/Team-Commonly#1992. The clause is deleted rather than patched, and Team-Commonly#1994 joins the other three as a fourth witness: one cause with four measurements beats two causes where one is refuted by `rev-list`. On the shape claim itself, measured over main: all 200 of its most recent commits have exactly one parent, and the newest two-parent commit is dc9d849 (2026-04-07, 1766 commits back), so "the merge queue's shape" describes something this repo stopped producing in April. The rule's earned text now carries the correction, because the false claim was published in the rule that exists to catch names and tests that do not measure what they say they measure. Guard, both modes: 44 rules, numbers 1..44 ascending with no gap, 28 citations all resolve, no rule changed its number or its name. * docs(checklist): the 60-character name pin is a window, not a wall (TASK-190) sprint-review measured that fingerprint() ends .slice(0, 60), so the guard binds only the first 60 characters of a rule's lead — and rule 32's claim that such a correction has no in-place mechanism was therefore false. Reproduced with a paired control on the real script: editing rule 32's lead from character 61 stays green (44 rules, no rule changed its name, exit 0); editing it at character 3 reds with the collision error. 40 of the file's 44 leads exceed 60 characters; 2,948 characters of lead text sit outside the pin. States the boundary and rescopes TASK-198 to the question it actually raises. * docs(checklist): name the base, and say what the two heads actually were (TASK-190) Two findings from sprint-review's gate, both open in the text rather than refuted by it. (b) rule 32 prescribed the patch-id proof without naming the base, and the only guidance for it sat three sentences away framed around a different trap. The base is now named where the proof is prescribed: BASE=$(git merge-base <carried-head> <merge>), the value actually run on all four of today's landings. (a) rule 44 called 5829815 and 1a811d8 'separate takes on the same change'. They are stronger than that: both single-commit diffs return patch-id 81ecc06228fb, the patch-id the landing itself was verified by — the same change on two bases, which is precisely why the stale stamp was seductive. * docs(checklist): the pin counts normalised characters, so name the raw offset (TASK-190) sprint-review's non-blocking addition, folded while the stamp is unspent rather than deferred: fingerprint() strips ** and `*_ and collapses whitespace BEFORE it slices, so the 60 are counted on the normalised lead and the boundary lands at a different raw offset in every lead carrying markup. Measured: rule 35's pin ends at raw character 64, rule 41's at 62, rule 13's at 61. Counting raw lengths gives 2,978 where the pin's own measure gives 2,948 -- the re-derivation error this sentence closes. * docs(checklist): the pin can skip over 60 — rule 41's raw offset does not exist Reviewer's finding: the sentence gave rule 41's raw offset as 62, computed as the last offset before the boundary while rules 35 and 13 were given as the offset at which the normalised count reaches 60 — one sentence, two quantities. Reproduced here on the file's own parseRules/fingerprint, and it is worse than a wrong number: fingerprint() ends .trim(), so a prefix that ends on a space has that space discounted and the next raw character makes it interior and counted. The normalised length can therefore step 59 -> 61 straight over the boundary, and four leads have NO raw offset that normalises to exactly 60: rules 3, 23 and 42 (raw 60 -> 61) and rule 41 (raw 62 -> 63). Exact landings: 34 leads at 60, rule 13 at 61, rule 35 at 64. Guard, CI's mode: 44 rules, 1..44, 38 citations resolve, no rule changed its number or its name. * docs(checklist): state the pin's raw offsets as a set, and the renumber clause's real cause ux-lead's docs gate on the previous head measured three false sentences. The pin does not land *between* two raw offsets in rules 3, 23, 41 and 42 — it lands on one: mutating a character either side of the boundary shows the pin ending at raw 60 in 37 of the 40 long leads, at 61 in rule 13, at 62 in rule 41 and at 64 in rule 35. The 59 -> 61 step is real but belongs to a prefix-by-prefix re-derivation, where `.trim()` discounts a trailing space; in the full lead that space is interior and counted. The sentence now gives the four end offsets, which add to 40, and moves the step to the sentence about re-deriving. "every lead that carries markup" is false for rules 14, 26 and 33, whose markup sits after the boundary and which end at raw 60 — now "every lead whose markup sits before it". Rule 43 attributed the 42/43 -> 43/44 discrepancy to Team-Commonly#2008 merging between the gate and the press. It did not: Team-Commonly#2008 merged at 15:35:29Z, both clearing gates are at `b32428b9` (15:39 and 15:41), and that head's file already reads 43 and 44. What stayed stale is the title and the first commit's subject, which the squash composed. Also: the two-parent commit count now names the base it was measured from (`50a454b0`), and the collision error is cited to TASK-198 rather than to "above", where it was never quoted. * docs(checklist): the mutation direction was inverted, and a re-derived count is not a control sprint-review's second gate found one sentence false: the method this paragraph hands a reader. "An edit to the last counted character leaves it green, and one character earlier reds it" is backwards. The pin's last counted character is inside the pin, so editing it reds the guard, and a character later is outside it and green. Measured on rule 41, whose pin's last counted character is the space at raw 62: raw 61 and 62 red, 63 and 64 green. A method given with its direction inverted lands the reader one character past the boundary it is being used to find, which is how the wrong number got published in the first place. The paragraph's other numbers stand and are re-measured here from the lead text: the pin ends at raw 60 in 37 of the 40 long leads, 61 in rule 13, 62 in rule 41, 64 in rule 35, and the prefix-by-prefix steps are at raw 60->61 in rules 3, 23 and 42 and at raw 62->63 in rule 41. Also stated, because the retraction is the more useful finding: a count re-derived with the instrument under test is not a control. The prefix-by-prefix re-derivation was gated twice and passed, confirming the error rather than the boundary. The arm that refutes a boundary claim is the mutation in the direction the claim asserts, and the check on a re-derivation is a differently-derived number.
1 participant
Sam, 2026-09-27: "the recent chat pop up is not accurate for the timestamp". This is the surface, and the fix.
The surface: the sidebar row time (
v2-pods__row-time) — every row in Recent and Everything.podMessageTime(pod)comes only from the lastGET /api/pods, and nothing re-read that list after mount; the minute tick atV2PodsSidebar.tsx:227recomputed the label around a frozen datum. So a fresh load was right (which is why lily's 03:37Z check found/api/podsequal to the truemax(created_at)), and a tab left open read further from the truth the longer it stayed open, while looking live.What changes. The minute tick also re-reads the datum, and so does becoming visible — the moment a row is actually read.
useV2Pods.refreshtakes{ silent }: it swaps the data without touchingloading/error, because the sidebar rendersv2-spinnerwhile loading, so a non-silent poll would blink the whole list once a minute, and a single failed poll would replace the last good rows with an error message. Foreground only: a hidden tab issues no request. (It still keeps the interval and still recomputes the label — "no network", not "no work"; that was true before this PR too.)What this is, and what it is not. This is remedy (c) from the TASK-184 row — polling + visibility, the pattern shipped one surface over in
V2ConnectorsPage(#1728/TASK-131) — chosen because the row's routing question (press #1751vsland the visibilitychange refetch now) went unanswered across three lease lapses and the pod focus names "fix visible timestamps that stay stale until refresh" as a follow-up. Remedy (b) is still the right architecture: a user-scopedpods_updatedpush, fed into thepatchLastMessagethat already exists with zero callers, makes a row current in seconds rather than within a minute. It needsjoinUserRoom, which is not on main — it is #1751, open since 2026-09-18, 13 files, all 15 checks green. If lily-shen would rather press that and make this row its second consumer, this commit is one revert and I will move the tests onto the push path.Named boundary, counted rather than omitted: the poll is one
GET /api/podsper minute per visible tab — the endpoint the shell already calls on mount. If that load is unwelcome, the visibility half alone still fixes the read-on-return case and is a one-line deletion. Also not fixed here: the sidebar constructs a seconduseV2Pods()even when the layout passes one, so a shell mount issues two initial/api/podsrequests — pre-existing, adjacent, and out of this commit's scope. Thev2-layout-invariantspin that the sidebar must not useuseV2Unreadis untouched: the unread badge stays on the attention collection, and this changes only the time half.Tests —
V2PodsSidebar.timestampFreshness.test.tsxdrives the surface (the label moves from2hto1mwhen the datum does; a hidden tab does not poll and re-reads the moment it becomes visible; 59s is not a tick) anduseV2PodsSilentRefresh.test.tsxcovers the option (silent keepsloadingfalse while in flight and keeps the last good rows on failure; the loud default is asserted beside it, so the silent arm cannot pass vacuously).The gate, fixed (
8a972c93). @sprint-review measured the cell that mattered and the first draft failed it: mount fails →erroris set → the sidebar renders the message INSTEAD of the rows (:501/:502) → every 60s the silent poll succeeded, swapped the data, and could not clear the one piece of state hiding the list. A poll fetching data nobody can see. A successful read is positive evidence the stored error is stale, so the success path now clears it in both modes. The invariant that mattered is intact: a failed poll still never sets an error, so a transient failure cannot replace the last good rows — and the complement is tested, because a silent failure must not clear an error it did not earn. New suiteV2PodsSidebar.pollRecovery.test.tsxmocks the API layer rather than the hook, so the realuseV2Podsand the real sidebar run end to end: error and no rows, then a successful tick brings the rows back with no reload.Mutation-verified (M7 is the gate cell), each red on the arm it belongs to: the tick stops re-reading ✕3 · drop the visibility guard ✕1 · call
refresh()without{silent}✕2 · ignore the silent flag in the hook ✕3 · a failed silent read surfaces the error ✕2 · retime the tick to 30s ✕1 · success no longer clears a stale error ✕1 surface + ✕1 hook, with the "error stays while the poll keeps failing" arm green beside it (so that suite is not just asserting that errors vanish). Note on the instrument: the first M7 probe was non-discriminating —setError(null)appears twice with identical text, and it hit the loud prelude, which no new arm depends on; retargeted to the success path, it reds. The surface suites stay green under the hook mutants by design — they mock the hook, so the option's semantics are pinned in the hook suite and the caller's obligation ({silent:true}) in the surface suite.Verification:
118/118 suites, 1033 tests·tsc --noEmitexit 0 · eslint 0 errors · no copy touched.Also noted, not fixed here:
patchLastMessageremains dead (3 refs, all insideuseV2Pods.ts), so the surface now has a poll AND an unused push lever. If #1751 lands and this becomes its second consumer, that is the moment to wire it or delete it.Gates:
@sprint-reviewthe code (delta above);@ux-leadthe behaviour — leave a tab open while another account posts, and confirm at 1200/390 that the list never blinks into the spinner on the minute.UX gate fix (
b8048c72)ux-lead's gate on
8a972c93found one thing, and it is a consequence of this PR: with Everything open and scrolled, a message to a pod in view moved the list 14 rows — 4 of 17 rows stayed in view — because the new poll re-reads/api/podsand rows re-sort bylastMessage..v2-pods__listnow declaresoverflow-anchor: none, which holds 16 of 17. The other two asks passed at 1200 and 390 (no blink on the minute; rows return after a failed load with no reload).The guard lives in
v2-layout-invariants.test.tsand reads the last.v2-pods__listblock on purpose: the selector is declared twice inv2.css(1096, from #251; 10997, from #1578 "sidebar at scale"), both top-level, so the later block wins every conflicting declaration and the first is fully superseded. The guard asserts that distinction (lastRuleBody !== ruleBody) so that removing the duplicate forces a re-read instead of silently switching which block is pinned, and it assertsoverflow-y: autois still present before asserting anchoring is off — a reset guard whose rule has vanished proves nothing.Verified at
b8048c72: 118 suites / 1034 tests ·tsc --noEmitexit 0 · eslint 0 errors (the file's 6 warnings are identical ate304c4cd, measured on both) · mutation: deleting the declaration reds the new test, and mutating only the superseded block stays green.